Skip to content
This repository was archived by the owner on Oct 27, 2020. It is now read-only.

fix(index): this of stat method is undefined #47

Merged
merged 3 commits into from
Oct 31, 2018
Merged

fix(index): this of stat method is undefined #47

merged 3 commits into from
Oct 31, 2018

Conversation

whxaxes
Copy link
Contributor

@whxaxes whxaxes commented Oct 31, 2018

fixed this issue: #46

@jsf-clabot
Copy link

jsf-clabot commented Oct 31, 2018

CLA assistant check
All committers have signed the CLA.

@whxaxes whxaxes changed the title fix: fixed this of stat method is undefined fix: this of stat method is undefined Oct 31, 2018
@whxaxes whxaxes mentioned this pull request Oct 31, 2018
@codecov
Copy link

codecov bot commented Oct 31, 2018

Codecov Report

Merging #47 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #47   +/-   ##
=======================================
  Coverage   17.14%   17.14%           
=======================================
  Files           2        2           
  Lines         105      105           
  Branches       15       15           
=======================================
  Hits           18       18           
  Misses         73       73           
  Partials       14       14
Impacted Files Coverage Δ
src/index.js 17.3% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 97be0c1...8805831. Read the comment docs.

@alexander-akait
Copy link
Member

@whxaxes Please accept CLA

@whxaxes
Copy link
Contributor Author

whxaxes commented Oct 31, 2018

@evilebottnawi done

@michael-ciniawsky michael-ciniawsky changed the title fix: this of stat method is undefined fix(index): this of stat method is undefined Oct 31, 2018
Copy link
Member

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@whxaxes Thx

src/index.js Outdated
@@ -41,9 +41,9 @@ function loader(...args) {
// this.fs can be undefined
// e.g when using the thread-loader
// fallback to the fs module
const stat = this.fs ? this.fs.stat : fs.stat;
const tempFs = this.fs || fs;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const tempFs = this.fs || fs;
const FS = this.fs || fs;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michael-ciniawsky FS? Why in uppercase? Why not usingFs or something related?

src/index.js Outdated
const toDepDetails = (dep, mapCallback) => {
stat(dep, (err, stats) => {
tempFs.stat(dep, (err, stats) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
tempFs.stat(dep, (err, stats) => {
FS.stat(dep, (err, stats) => {

src/index.js Outdated
@@ -112,9 +112,9 @@ function pitch(remainingRequest, prevRequest, dataInput) {
callback();
return;
}
const stat = this.fs ? this.fs.stat : fs.stat;
const tempFs = this.fs || fs;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const tempFs = this.fs || fs;
const FS = this.fs || fs;

src/index.js Outdated
async.each(cacheData.dependencies.concat(cacheData.contextDependencies), (dep, eachCallback) => {
stat(dep.path, (statErr, stats) => {
tempFs.stat(dep.path, (statErr, stats) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
tempFs.stat(dep.path, (statErr, stats) => {
FS.stat(dep.path, (statErr, stats) => {

@whxaxes
Copy link
Contributor Author

whxaxes commented Oct 31, 2018

@michael-ciniawsky done

@michael-ciniawsky
Copy link
Member

Released in v1.2.5

@michael-ciniawsky michael-ciniawsky removed this from the 1.2.5 milestone Oct 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants